Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Doc prompt #1031

Merged
merged 1 commit into from
Mar 20, 2015
Merged

Doc prompt #1031

merged 1 commit into from
Mar 20, 2015

Conversation

EricGebhart
Copy link
Contributor

This is changes the behavior of the doc functions to always prompt with a default of symbol at point. The prompts were given contextual labels instead of 'Symbol:' where needed.

@bbatsov
Copy link
Member

bbatsov commented Mar 19, 2015

Please, take a look at CONTRIBUTING.md (I'm referring to the commit message format in particular).

@bbatsov
Copy link
Member

bbatsov commented Mar 19, 2015

Another thing - I can imagine some people actually like the current behaviour, so I'd suggest a config option to toggle between the two.

@vspinu
Copy link
Contributor

vspinu commented Mar 19, 2015

I'd suggest a config option to toggle between the two.

That might be tricky in light of the proposed other-window prefix usage for the jump-to-var(#1014).

@EricGebhart
Copy link
Contributor Author

OK. sorry about that.

I thought about an option to turn it on and off, but that does prevent more useful prefix behaviors like jump to var other window. This does cause the behavior to match that of many other emacs packages, where the current behavior is mostly frustrating. Hitting return once is a lot easier than typing C-u prefix after figuring out that is what you need to do.

@bbatsov
Copy link
Member

bbatsov commented Mar 19, 2015

I thought about an option to turn it on and off, but that does prevent more useful prefix behaviors like jump to var other window. This does cause the behavior to match that of many other emacs packages, where the current behavior is mostly frustrating. Hitting return once is a lot easier than typing C-u prefix after figuring out that is what you need to do.

Fair enough. Let's see the feedback we'll get. For now just update the commits and mention this in the changelog.

@EricGebhart
Copy link
Contributor Author

What is your preference for connecting the changelog, the issue and the pull request together?

@expez
Copy link
Member

expez commented Mar 19, 2015

@EricGebhart I opened an issue about jump-to-var-other-window the other day, here. It'd be awesome if you had the time to fix this! I'd do it myself but I'm busy working on classloader isolation for clj-refactor atm.

@EricGebhart
Copy link
Contributor Author

@expez That one is on my radar. I'm in the middle of all that code anyway. Learning my way around...

@bbatsov
Copy link
Member

bbatsov commented Mar 19, 2015

[Fix #699] Changed interactive doc functions to give prompt. -> [Fix #699] Prompt for symbol in interactive doc functions (use present tense, no . in commit message title)

@bbatsov
Copy link
Member

bbatsov commented Mar 20, 2015

@EricGebhart Commit message titles are titles (meaning they should not end with .). :-)

I'm sorry if I sound picky, but I care about every detail.

cider-doc, cider-grimoire, jump-to-var and other functions using
cider-read-symbol-name were not interactive without a prefix or
missing symbol.  cider-read-symbol-name now prompts with the
symbol at point as a default.
@EricGebhart
Copy link
Contributor Author

It's ok. All the periods are gone. I'll be sure to leave them off in the future.

bbatsov added a commit that referenced this pull request Mar 20, 2015
@bbatsov bbatsov merged commit 012ccb3 into clojure-emacs:master Mar 20, 2015
@bbatsov
Copy link
Member

bbatsov commented Mar 20, 2015

👍

@malcolmsparks
Copy link

Please revert this commit, for the reasons I give in the thread in #1014

@grammati
Copy link
Contributor

@bbatsov, since you said above "Let's see the feedback we'll get" - I second @malcolmsparks . Sorry, but I really hate the new behavior. Having to type RET after every M-. is very clunky.

@j0ni
Copy link

j0ni commented Mar 25, 2015

I also am finding this rather annoying. I understand there are arguments for this behaviour, so I would settle for the ability to configure it.

@cichli
Copy link
Member

cichli commented Mar 29, 2015

You can now set cider-prompt-for-symbol to nil to disable the prompting behaviour. Passing a prefix argument to any of the source/doc lookup commands will make them do the opposite of that config option, if you wish to override it for just one command.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants